-
Notifications
You must be signed in to change notification settings - Fork 63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updated VCR from openshift repo, updated quota & volume specs #216
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
243657c
to
d80a96a
Compare
@zeari @yaacov @moolitayer PTAL. |
@@ -392,13 +392,17 @@ def tag_in_category_with_description(category, description) | |||
:match_requests_on => [:path,]) do # , :record => :new_episodes) do | |||
EmsRefresh.refresh(@ems) | |||
end | |||
|
|||
# fake node that should get archived on later refresh | |||
@archived_node = FactoryGirl.create(:container_node, :name => "node", :ems_id => @ems.id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see you just moved this one around, Is @archived_node used somewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #209 (comment) for benefit of moving, fits with other archived counts.
instance var unused, let me drop it 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dropped instance var
let(:container_volumes_count) { 22 } | ||
let(:persintent_volumes_count) { 2 } | ||
let(:container_volumes_count) { 21 } | ||
let(:persintent_volumes_count) { 103 } # 3 from our template + minishift creates pv0001..pv0100 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be cleaner to remove the extra 10 volumes in the future.
When someone reproduces a VCR it should be the same for minishift, origin, kubernetes etc. It would be good to assume an empty environment. Is it possible to delete the volumes in the beginning?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice idea.
oc delete --ignore-not-found pv $(printf "pv%s " $(seq -w 0001 0100))
Do you want me to re-record?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done (implemented in ManageIQ/manageiq-providers-openshift#83, copied new VCR, count is now 3).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Script deletes (and refresh should archive): po/my-build-config-0-1-build po/my-pod-0 + po/my-build-config-1-1-build po/my-pod-1 + po/my-pod-2
Checked commits cben/manageiq-providers-kubernetes@ea58a4f~...e62556c with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0 **
|
addressed review & rubocop. good to go from my side. |
Updated VCR from openshift repo, updated quota & volume specs (cherry picked from commit ca27fb7) https://bugzilla.redhat.com/show_bug.cgi?id=1559544
Gaprindashvili backport details:
|
VCR files copied from
ManageIQ/manageiq-providers-openshift#83
(can be merged independently but let's review both first).
Volumes: no longer rely on VCR happening to include Hawkular &
Cassandra, test specific volumes I added to template.
Quotas: test current in-place update, no archiving behavior.
(To be changed in later PR.)
https://bugzilla.redhat.com/show_bug.cgi?id=1504560
@miq-bot add-label test, gaprindashvili/yes
@zeari @yaacov @moolitayer please review (recommend by commits)